Mysqli: fix missing error for invalid mysqli_options() option by arshidkv12 · Pull Request #20971 · php/php-src

arshidkv12 · 2026-01-19T03:13:43Z

#20968

…cting report_mode

iluuu1994

From what I can see (as somebody who doesn't maintain this code), most arguments are validated without the MYSQLI_REPORT_ERROR guard, and throw a zend_argument_value_error().

But somebody who does maintain this should confirm.

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

…cting report_mode

arshidkv12 · 2026-01-19T16:18:24Z

From what I can see (as somebody who doesn't maintain this code), most arguments are validated without the MYSQLI_REPORT_ERROR guard, and throw a zend_argument_value_error().

But somebody who does maintain this should confirm.

- if (MyG(report_mode) & MYSQLI_REPORT_ERROR) {
	zend_value_error("mysqli_options(): Invalid option %d", (int)mysql_option);

Do we need to remove if (MyG(report_mode) & MYSQLI_REPORT_ERROR)? What do you think?

iluuu1994 · 2026-01-19T17:07:10Z

Somebody else should review this. /cc @Girgias @SakiTakamachi

kamil-tekiela · 2026-01-19T17:28:21Z

First of all, I don't know if this should be an error condition. I guess it kind of makes sense. But as you can see in the test, no message was the behaviour by design.

The error should be without the guard and without the function name in the message. It seems to me like you have also put it in the wrong place. Judging by the message and the test, you wanted to put it in the switch statement after that.

The message isn't very descriptive either. What does it mean "invalid option"? What should the user do differently?

…cting report_mode

arshidkv12 · 2026-01-20T05:45:34Z

Changed the error message to zend_value_error("Argument #1 ($option) is not a valid mysqli option");

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

…cting report_mode

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

arshidkv12 · 2026-01-20T13:35:14Z

Thank you @kamil-tekiela kamil-tekiela

kamil-tekiela · 2026-01-20T13:37:06Z

@iluuu1994 Are you ok with how this looks now?

…cting report_mode

iluuu1994

I'm not a codeowner, but yes, LGTM now. Thanks @arshidkv12!

…cting report_mode

Girgias · 2026-01-20T17:54:26Z

@@ -0,0 +1,27 @@
+--TEST--
+Bug #20968 mysqli_options() with invalid option triggers ValueError


Suggested change

Bug #20968 mysqli_options() with invalid option triggers ValueError

GH-20968 mysqli_options() with invalid option triggers ValueError

We use Bug #xxxxx notation to refer to old bugsnet bugs.

Girgias · 2026-01-20T17:54:59Z

+
+$mysqli = new mysqli($host, $user, $passwd, $db, $port, $socket);
+
+$value = $mysqli->options(10, 'invalid_option');


Add a try catch here and check the exception message, we don't care about the stack trace.

Girgias · 2026-01-20T17:56:47Z

@@ -0,0 +1,27 @@
+--TEST--
+Bug #20968 mysqli_options() with invalid option triggers ValueError


Suggested change

Bug #20968 mysqli_options() with invalid option triggers ValueError

Bug #20968 mysqli_options() with invalid option should triggers ValueError

The phrasing of the bug title seems to imply that an invalid option MUST NOT thow a ValueError, which is the complete opposite of what is being done.

…cting report_mode

arshidkv12 · 2026-01-22T10:14:46Z

Please check the new commit @Girgias

Co-authored-by: Gina Peter Banyard <girgias@php.net>

…cting report_mode

Girgias

Minor nits, but otherwise LGTM now.

Girgias · 2026-01-24T11:43:05Z

+
+?>
+--EXPECTF--
+mysqli::options(): Argument #%d ($option) must be MYSQLI_INIT_COMMAND, MYSQLI_SET_CHARSET_NAME,  MYSQLI_SERVER_PUBLIC_KEY, or one of the MYSQLI_OPT_* constants


Nit: EOL before EOF.

Co-authored-by: Gina Peter Banyard <girgias@php.net>

…cting report_mode

arshidkv12 · 2026-02-02T13:12:41Z

Could you please merge this PR?

bukka · 2026-02-16T16:18:06Z

@Girgias How it this different than other similar changes that are still waiting for that decision about the strategy for creating new ValueErrors...

Girgias · 2026-02-17T18:28:22Z

@Girgias How it this different than other similar changes that are still waiting for that decision about the strategy for creating new ValueErrors...

I have yet to hear a good argument why allowing users from writing buggy code is good, and why erroring on invalid values is bad.

bukka · 2026-02-18T23:46:04Z

This is more about breaking existing code and the fact that we have a policy that requires RFC for BC breaks.

@arshidkv12 Please can you add it to your RFC for ValueErrors. We can leave it merged for now but if it is not accepted, it will need to be reverted.

arshidkv12 · 2026-02-18T23:59:32Z

I will include it in the RFC for ValueErrors.

arshidkv12 added 2 commits January 19, 2026 08:35

Fix bug gh20968

a401699

mysqli: raise ValueError for invalid option in mysqli_options() respe…

217b19b

…cting report_mode

arshidkv12 requested review from bukka and kamil-tekiela as code owners January 19, 2026 03:13

github-actions Bot added the Extension: mysqli label Jan 19, 2026

arshidkv12 added 5 commits January 19, 2026 08:52

mysqli: raise ValueError for invalid option in mysqli_options() respe…

66fbc0a

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

54a8e06

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

0716f97

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

d7dd938

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

1aa360a

…cting report_mode

iluuu1994 reviewed Jan 19, 2026

View reviewed changes

Comment thread ext/mysqli/mysqli_api.c Outdated

Comment thread ext/mysqli/tests/gh20968.phpt Outdated

Comment thread ext/mysqli/mysqli_api.c Outdated

arshidkv12 and others added 3 commits January 19, 2026 21:33

Update ext/mysqli/mysqli_api.c

04f8006

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

Update ext/mysqli/tests/gh20968.phpt

9ba4bf2

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

2db6e98

…cting report_mode

arshidkv12 requested a review from iluuu1994 January 19, 2026 16:50

arshidkv12 added 3 commits January 20, 2026 11:09

mysqli: raise ValueError for invalid option in mysqli_options() respe…

29ed278

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

1647ae8

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

0a98c9c

…cting report_mode

kamil-tekiela reviewed Jan 20, 2026

View reviewed changes

Comment thread ext/mysqli/mysqli_api.c Outdated

kamil-tekiela reviewed Jan 20, 2026

View reviewed changes

Comment thread ext/mysqli/tests/mysqli_options.phpt Outdated

kamil-tekiela reviewed Jan 20, 2026

View reviewed changes

Comment thread ext/mysqli/tests/mysqli_set_opt.phpt Outdated

arshidkv12 and others added 4 commits January 20, 2026 18:59

Update ext/mysqli/mysqli_api.c

8193542

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

Update ext/mysqli/tests/mysqli_options.phpt

3a633b3

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

072d1a4

…cting report_mode

Update ext/mysqli/tests/mysqli_set_opt.phpt

a188868

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

e102967

…cting report_mode

iluuu1994 approved these changes Jan 20, 2026

View reviewed changes

Comment thread ext/mysqli/tests/mysqli_set_opt.phpt Outdated

kamil-tekiela approved these changes Jan 20, 2026

View reviewed changes

mysqli: raise ValueError for invalid option in mysqli_options() respe…

6a71d4f

…cting report_mode

Girgias requested changes Jan 20, 2026

View reviewed changes

kamil-tekiela mentioned this pull request Jan 20, 2026

Add MYSQLI_OPT_COMPRESS #20987

Merged

mysqli: raise ValueError for invalid option in mysqli_options() respe…

d1df211

…cting report_mode

arshidkv12 and others added 2 commits January 23, 2026 07:19

Update ext/mysqli/mysqli_api.c

d335402

Co-authored-by: Gina Peter Banyard <girgias@php.net>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

69f0924

…cting report_mode

arshidkv12 requested a review from Girgias January 23, 2026 18:32

Girgias approved these changes Jan 24, 2026

View reviewed changes

arshidkv12 and others added 3 commits January 24, 2026 17:17

Update ext/mysqli/mysqli_api.c

81bd296

Co-authored-by: Gina Peter Banyard <girgias@php.net>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

e754b49

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

b16f994

…cting report_mode

arshidkv12 requested a review from Girgias January 24, 2026 12:37

Girgias reviewed Jan 24, 2026

View reviewed changes

Comment thread ext/mysqli/tests/gh20968.phpt Outdated

mysqli: raise ValueError for invalid option in mysqli_options() respe…

357164e

…cting report_mode

arshidkv12 requested a review from Girgias January 24, 2026 17:02

Girgias approved these changes Jan 24, 2026

View reviewed changes

Girgias merged commit a5a0ff6 into php:master Feb 3, 2026
10 checks passed

arshidkv12 deleted the mysql-option branch March 15, 2026 15:05

		@@ -0,0 +1,27 @@
		--TEST--
		Bug #20968 mysqli_options() with invalid option triggers ValueError

	Bug #20968 mysqli_options() with invalid option triggers ValueError
	GH-20968 mysqli_options() with invalid option triggers ValueError


		$mysqli = new mysqli($host, $user, $passwd, $db, $port, $socket);

		$value = $mysqli->options(10, 'invalid_option');

Conversation

arshidkv12 commented Jan 19, 2026

Uh oh!

iluuu1994 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arshidkv12 commented Jan 19, 2026

Uh oh!

iluuu1994 commented Jan 19, 2026

Uh oh!

kamil-tekiela commented Jan 19, 2026

Uh oh!

arshidkv12 commented Jan 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arshidkv12 commented Jan 20, 2026

Uh oh!

kamil-tekiela commented Jan 20, 2026

Uh oh!

iluuu1994 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Girgias Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Girgias Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Girgias Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

arshidkv12 commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

Girgias Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arshidkv12 commented Feb 2, 2026

Uh oh!

Uh oh!

bukka commented Feb 16, 2026

Uh oh!

Girgias commented Feb 17, 2026

Uh oh!

bukka commented Feb 18, 2026

Uh oh!

arshidkv12 commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

arshidkv12 commented Jan 22, 2026 •

edited

Loading